Fix typed throws testing crash by lawrence-forooghian · Pull Request #414 · ably/ably-chat-swift

lawrence-forooghian · 2025-10-13T20:31:37Z

Resolves #233.

Summary by CodeRabbit

Documentation
- Removed outdated guidance and edge-case workarounds about Swift typed throws from the contributing guide, simplifying contributor instructions.
Tests
- Simplified error assertions across message reactions, messages, presence, and rooms tests by capturing thrown errors directly and asserting once, reducing boilerplate and improving clarity.
- Minor import cleanup to support the revised testing pattern.

coderabbitai · 2025-10-13T20:31:56Z

Walkthrough

Documentation removed typed-throws workarounds; tests refactored to use direct await #expect(throws:) to capture thrown errors instead of nested closure patterns. DefaultRoomsTests also adds an import Ably. No public APIs or runtime behavior changed.

Changes

Cohort / File(s)	Summary
Documentation cleanup `CONTRIBUTING.md`	Removed guidance and workarounds related to Swift typed throws, disabling type inference for `do` blocks, explicit thrown-error typing, and examples/workarounds for the Swift Testing `#expect(throws:)` compiler-crash scenario.
Tests: message reactions & messages `Tests/AblyChatTests/DefaultMessageReactionsTests.swift`, `Tests/AblyChatTests/DefaultMessagesTests.swift`	Replaced nested two-step/nested-closure error assertions with direct `await #expect(throws: ...)` capturing the thrown `ARTErrorInfo`, then asserting equality against expected `ARTErrorInfo`.
Tests: presence `Tests/AblyChatTests/DefaultPresenceTests.swift`	Replaced nested-closure error patterns with `await #expect(throws: (any Error).self)` captures, followed by `isChatError(...)` assertions checking status codes and causes.
Tests: rooms + import `Tests/AblyChatTests/DefaultRoomsTests.swift`	Switched nested-closure error assertions to `await #expect(throws: ...)` with `isChatError(...)` checks; added top-of-file `import Ably`.

Sequence Diagram(s)

sequenceDiagram
  participant Test as Test Case
  participant Expect as #expect(throws:)
  participant SUT as System Under Test (function)
  Note over Test,Expect: New flow — capture thrown error directly
  Test->>Expect: await #expect(throws: ErrorType.self) { call SUT() }
  Expect->>SUT: invoke SUT()
  SUT-->>Expect: throws ARTErrorInfo / Error
  Expect-->>Test: returns thrownError
  Test->>Test: assert thrownError == expected / isChatError(...)
  note right of Test: simplifies from prior nested closure + nested expect pattern

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Poem

A rabbit hops through tests at night,
Removes the burrows, finds the light.
We catch the throw and check it true,
Docs trimmed down, imports anew.
Pip, hop, compile — the garden's right. 🐇✨

Pre-merge checks and finishing touches

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check	✅ Passed	The title succinctly captures the intent to address the compiler crash caused by typed throws in tests and aligns with the core change of removing the workaround for typed-throws tests.
Linked Issues Check	✅ Passed	The pull request removes the legacy two-stage closures and non-typed-throw workarounds in all affected tests, replaces them with the new await #expect(throws:) API to capture typed errors, and cleans up related documentation, fully addressing the removal of the workaround and leveraging the updated Xcode 16.3 testing API specified in issue #233.
Out of Scope Changes Check	✅ Passed	All modifications are directly tied to eliminating the typed-throws workaround and updating tests to use the new #expect(throws:) API, and the documentation edits remove outdated guidance, with no unrelated or extraneous changes introduced.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

✨ Finishing touches

📝 Generate docstrings

🧪 Generate unit tests (beta)

Create PR with unit tests
Post copyable unit tests in a comment
Commit unit tests in branch fix-throws-test-crash

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

Disabled knowledge base sources:

Jira integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between 5624f13 and 9172453.

📒 Files selected for processing (5)

CONTRIBUTING.md (0 hunks)
Tests/AblyChatTests/DefaultMessageReactionsTests.swift (3 hunks)
Tests/AblyChatTests/DefaultMessagesTests.swift (6 hunks)
Tests/AblyChatTests/DefaultPresenceTests.swift (6 hunks)
Tests/AblyChatTests/DefaultRoomsTests.swift (3 hunks)

💤 Files with no reviewable changes (1)

CONTRIBUTING.md

🚧 Files skipped from review as they are similar to previous changes (2)

Tests/AblyChatTests/DefaultMessageReactionsTests.swift
Tests/AblyChatTests/DefaultRoomsTests.swift

🧰 Additional context used

📓 Path-based instructions (1)

Tests/AblyChatTests/**/*.swift

📄 CodeRabbit inference engine (CLAUDE.md)

Tests/AblyChatTests/**/*.swift: Place all tests under Tests/AblyChatTests
Use test attribution tags: @SPEC, @specOneOf(m/n), @specPartial, @specUntested, @specNotApplicable with appropriate spec IDs and explanations
For Swift Testing #expect(throws:) with typed errors, move typed-throw code into a separate non-typed-throw function (until Xcode 16.3+)

Files:

Tests/AblyChatTests/DefaultMessagesTests.swift
Tests/AblyChatTests/DefaultPresenceTests.swift

🧬 Code graph analysis (2)

Tests/AblyChatTests/DefaultMessagesTests.swift (4)

Sources/AblyChat/DefaultMessages.swift (5)

send (126-132)

update (134-140)

delete (142-148)

get (151-157)

history (118-124)

Example/AblyChatExample/Mocks/MockClients.swift (15)

send (168-185)

send (275-292)

send (353-364)

update (187-205)

update (523-525)

update (527-529)

update (531-545)

delete (207-221)

delete (294-305)

get (29-36)

get (223-238)

get (471-481)

get (483-493)

get (600-602)

history (164-166)

Sources/AblyChat/Messages.swift (1)

historyBeforeSubscribe (447-449)

Sources/AblyChat/Subscription.swift (1)

historyBeforeSubscribe (94-108)

Tests/AblyChatTests/DefaultPresenceTests.swift (5)

Example/AblyChatExample/Mocks/MockClients.swift (12)

enter (499-501)

enter (503-505)

enter (507-521)

update (187-205)

update (523-525)

update (527-529)

update (531-545)

get (29-36)

get (223-238)

get (471-481)

get (483-493)

get (600-602)

Sources/AblyChat/DefaultPresence.swift (8)

enter (95-97)

enter (99-101)

enter (104-125)

update (127-129)

update (131-133)

update (136-157)

get (18-41)

get (43-66)

Sources/AblyChat/AblyCocoaExtensions/InternalAblyCocoaTypes.swift (5)

enter (275-289)

update (291-305)

get (223-239)

get (241-257)

get (437-440)

Tests/AblyChatTests/Mocks/MockRealtimePresence.swift (4)

enter (42-47)

update (49-54)

get (19-25)

get (27-33)

Tests/AblyChatTests/Helpers/Helpers.swift (1)

isChatError (7-33)

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (11)

GitHub Check: Example app, tvOS (Xcode 26.0)
GitHub Check: Example app, macOS (Xcode 26.0)
GitHub Check: Example app, iOS (Xcode 26.0)
GitHub Check: Xcode, release configuration, tvOS (Xcode 26.0)
GitHub Check: Xcode, release configuration, macOS (Xcode 26.0)
GitHub Check: Xcode, release configuration, iOS (Xcode 26.0)
GitHub Check: Xcode, tvOS (Xcode 26.0)
GitHub Check: SPM, release configuration (Xcode 26.0)
GitHub Check: Xcode, macOS (Xcode 26.0)
GitHub Check: Xcode, iOS (Xcode 26.0)
GitHub Check: SPM (Xcode 26.0)

🔇 Additional comments (12)

Tests/AblyChatTests/DefaultMessagesTests.swift (6)

187-191: Typed throw capture looks great

Directly grabbing the ARTErrorInfo via await #expect(throws:) keeps the assertion focused and drops the old helper boilerplate. ✅

205-213: Update path assertion is tidy

Same streamlined await #expect(throws:) pattern here—reads cleaner and still validates the exact error instance.

227-231: Delete path error check is spot on

The new typed-throws expectation neatly surfaces the ARTErrorInfo for equality without extra indirection.

293-297: Get-by-serial failure flow confirmed

Capturing the thrown ARTErrorInfo directly keeps this failure assertion concise and clear.

491-495: Subscription history error assertion cleaned up

The direct #expect(throws:) usage pairs well with the equality check and removes the prior wrapper noise.

543-547: History error handling modernized

This switch to capturing the thrown ARTErrorInfo inline preserves intent while leveraging the new Testing API.

Tests/AblyChatTests/DefaultPresenceTests.swift (6)

100-106: Failure capture uses the new API well

Grabbing the thrown error with await #expect(throws:) makes the follow-up isChatError check straightforward.

131-135: Invalid-state enter test nicely simplified

The expectation now directly yields the thrown value, keeping the chat-error verification tight.

207-213: Attach-failure update test reads cleaner

Returning the captured error from #expect removes the old closure dance without losing coverage.

238-242: Invalid-state update test stays explicit

The new pattern cleanly exposes the error so isChatError can assert the precise status.

344-347: Presence get failure handled idiomatically

Directly capturing the thrown error tightens the assertion and matches the surrounding tests’ style.

392-398: Attach failure during get now concise

This await #expect(throws:) usage keeps the failure mode obvious and avoids the prior extra indirection.

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

Resolves #233.

github-actions Bot temporarily deployed to staging/pull/414/AblyChat October 13, 2025 20:33 Inactive

lawrence-forooghian requested review from maratal and umair-ably October 13, 2025 20:33

umair-ably approved these changes Oct 15, 2025

View reviewed changes

lawrence-forooghian changed the base branch from main to 405-update-and-delete-accept-serial October 15, 2025 12:24

lawrence-forooghian force-pushed the 405-update-and-delete-accept-serial branch from bbcc60e to 103a812 Compare October 15, 2025 12:27

Base automatically changed from 405-update-and-delete-accept-serial to main October 15, 2025 12:50

Fix typed throws testing crash

9172453

Resolves #233.

lawrence-forooghian force-pushed the fix-throws-test-crash branch from 5624f13 to 9172453 Compare October 15, 2025 12:54

lawrence-forooghian enabled auto-merge October 15, 2025 12:55

github-actions Bot deployed to staging/pull/414/AblyChat October 15, 2025 12:55 View deployment

lawrence-forooghian merged commit efc5265 into main Oct 15, 2025
17 checks passed

lawrence-forooghian deleted the fix-throws-test-crash branch October 15, 2025 13:03

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix typed throws testing crash#414

Fix typed throws testing crash#414
lawrence-forooghian merged 1 commit intomainfrom
fix-throws-test-crash

lawrence-forooghian commented Oct 13, 2025 •

edited by coderabbitai Bot

Loading

Uh oh!

coderabbitai Bot commented Oct 13, 2025 •

edited

Loading

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Milestone

Development

Uh oh!

2 participants

Conversation

lawrence-forooghian commented Oct 13, 2025 • edited by coderabbitai Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary by CodeRabbit

Uh oh!

coderabbitai Bot commented Oct 13, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

Sequence Diagram(s)

Estimated code review effort

Poem

Pre-merge checks and finishing touches

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Milestone

Development

Uh oh!

2 participants

lawrence-forooghian commented Oct 13, 2025 •

edited by coderabbitai Bot

Loading

coderabbitai Bot commented Oct 13, 2025 •

edited

Loading